Skip to content

Conversation

schlndh
Copy link
Contributor

@schlndh schlndh commented Aug 9, 2024

This is a more advanced alternative to #3282
Fixes phpstan/phpstan#7707

I didn't go the route you suggested here: #3282 (comment) . Instead, I bypassed php8-stubs for these functions so that they don't conflict with the new stubs. And I added support for in-the-middle variadic parameter to FunctionCallParametersCheck. IMO handling it directly in the reflection should cause less inconsistency.

if (
(!$phpDocType instanceof NeverType || ($type instanceof MixedType && !$type->isExplicitMixed()))
&& $type->isSuperTypeOf(TemplateTypeHelper::resolveToBounds($phpDocType))->yes()
($type->isCallable()->yes() && $phpDocType->isCallable()->yes())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, I could broaden the type of the callbacks in function map to callable and then I wouldn't need this. I'm not sure what's better.

Comment on lines +191 to +210
|| (
$className === null
// These functions have a variadic parameter in the middle. This is not well described by php8-stubs.
&& in_array(
$functionName,
[
'array_diff_uassoc',
'array_diff_ukey',
'array_intersect_uassoc',
'array_intersect_ukey',
'array_udiff',
'array_udiff_assoc',
'array_udiff_uassoc',
'array_uintersect',
'array_uintersect_assoc',
'array_uintersect_uassoc',
],
true,
)
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to complicate it with the php8-stubs. The issue is that the stubs look like this:

/** @param array|callable $rest */
function array_intersect_uassoc(array $array, ...$rest): array
{
}

which then overrides the newly introduced stub. So I needed to skip them somehow. I suppose an alternative would be to skip php8-stubs where the last parameter is ...$rest, but I prefer this more explicit way.

@ondrejmirtes
Copy link
Member

Hi, I appreciate your effort but I'm not really sure about this approach. This isn't really valid PHP:

function array_udiff(
    array $array,
    array ...$arrays,
    callable $value_compare_func,
): array {}

So stubs shouldn't be written like this and info with this structure should not appear in reflection. The fact you need to handle this situation in src/Rules/FunctionCallParametersCheck.php (in the rules layer) is a sign we might break a bunch of code that would forget to handle it.

To modify the signature on the fly based on the actual call args with my suggestion from here #3282 (comment) would be much more palatable for me.

@schlndh schlndh closed this Aug 19, 2024
@schlndh schlndh deleted the feature-arrayUserCallbackTypesV2 branch August 19, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants